Skip to content

Conversation

@Avasam
Copy link
Collaborator

@Avasam Avasam commented Oct 19, 2025

Mostly updating obsolete documentation and simplifying some tests. I searched for the terms "Windows 2000" and "Vista".

Along with #2668, this removes all usages of sys.getwindowsversion(), pushing the Python code to be more Windows-version agnostic.

Most of the actual code changes had already been done in:

@Avasam Avasam force-pushed the Remove-considerations-for-Windows-2000-and-Windows-Vista branch from 2850ae8 to b64507f Compare October 19, 2025 00:41
@Avasam Avasam force-pushed the Remove-considerations-for-Windows-2000-and-Windows-Vista branch from b64507f to e1d0f07 Compare October 19, 2025 01:11
@Avasam Avasam force-pushed the Remove-considerations-for-Windows-2000-and-Windows-Vista branch from e1d0f07 to 3c3a6e9 Compare October 19, 2025 01:14
@Avasam Avasam requested a review from mhammond October 19, 2025 01:17
#define CHECK_PFN(fname) \
if (pfn##fname == NULL) \
return PyErr_Format(PyExc_NotImplementedError, "%s is not available on this platform", #fname);
// Not available on Vista or earlier
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't hate these comments because they help explain why there is a function pointer used. Removing the comment removes all context. I'm fine with updating the comment to be current and supply the context, or to remove the function pointer entirely.

Copy link
Collaborator Author

@Avasam Avasam Oct 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll add back or modify the comment.
Edit: this file only contained a single CHECK_PFN usage, removed along with comment.

Anyway I realized it later, but I think all these CHECK_PFN macros can be removed (looks like they're all for functions that should now always exist at build and runtime).

But I'd do that in its own PR. The main concern there will be to not accidentally remove cygwin workaround/support.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anyway I realized it later, but I think all these CHECK_PFN macros can be removed (looks like they're all for functions that should now always exist at build and runtime).

Right - I was trying to suggest that this means we can remove the function pointers entirely rather than just the null checks.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right - I was trying to suggest that this means we can remove the function pointers entirely rather than just the null checks.

that's what I ended up doing. Which rendered the line CHECK_PFN(SHGetPropertyStoreForWindow) dead code, so I removed it. Making the macro CHECK_PFN (in this file) unused, so I removed it.

// interface.
static PyObject *PyAssocCreateForClasses(PyObject *self, PyObject *args)
{
// @comm This function is only available on Vista and later; a
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kinda like the above, I think I'd rather just avoid using the function pointer entirely (and ditto below)

Microsoft released a patch for handling time zones in 2007 (see
https://learn.microsoft.com/en-us/troubleshoot/windows-client/system-management-components/daylight-saving-time-help-support)
As a result, patched systems will give an incorrect result for
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"will" seems more correct than "would" here. I get that maybe you are trying to refer to the past, but if any of those systems are still running and are patched, they will do what's described, right?

Copy link
Collaborator Author

@Avasam Avasam Oct 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but if any of those systems are still running and are patched, they will do what's described, right?

Correct, although any of those systems can't run pywin32 anymore (all systems that can run pywin32 are vista-or-later). I kept the tests for coverage sake.

In french we have a verb tense specifically for this scenario ^^" (conditional past).

Anyway I don't mind changing back to will rather than would/would have.

@Avasam Avasam requested a review from mhammond October 22, 2025 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants